Orchestration stack subcollection #14273
Conversation
spec/requests/api/services_spec.rb
Outdated
| before do | ||
| svc.add_resource!(ot, :name => ResourceAction::PROVISION) | ||
| end | ||
|
|
There was a problem hiding this comment.
probably add the test for failed get (forbidden)
|
LGTM!! 😍 minor test to add, Thanks!! /cc @imtayadeway |
| module Subcollections | ||
| module OrchestrationStacks | ||
| def orchestration_stacks_query_resource(object) | ||
| object.respond_to?(:orchestration_stacks) ? object.orchestration_stacks : [] |
There was a problem hiding this comment.
Are there services that don't have orchestration stacks? If so, can you add a test to verify what happens when it doesn't?
config/api.yml
Outdated
| :orchestration_stacks_subcollection_actions: | ||
| :get: | ||
| - :name: read | ||
| :identifier: orchestration_stack_view |
There was a problem hiding this comment.
Since these identifiers don't seem linked in any way to the parent services collection, you could put this configuration under the orchestration stacks section. But not a blocker - I just prefer to think of this section as configuration that's tailored for this parent collection 😄
spec/requests/api/services_spec.rb
Outdated
| @@ -625,4 +625,46 @@ def expect_svc_with_vms | |||
| end | |||
| end | |||
| end | |||
|
|
|||
| describe 'Orchestration Stack subcollection' do | |||
| let(:ot) { FactoryGirl.create(:orchestration_stack) } | |||
There was a problem hiding this comment.
Just curious, but why "ot"?
|
@jntullo LGTM, I think just needs one additional test as per https://github.com/ManageIQ/manageiq/pull/14273/files#r105734504 👍 |
spec/requests/api/services_spec.rb
Outdated
| run_get("#{services_url(svc1.id)}/orchestration_stacks", :expand => 'resources') | ||
|
|
||
| expected = { | ||
| 'resources' => [] |
There was a problem hiding this comment.
Is this because service.orchestration_stacks # => [] Or because service.orchestration_stacks # => NoMethodError? It looks like the former from what I understand, so wondering if the respond_to? check is needed above?
There was a problem hiding this comment.
Ah, I see! Okay, since this is not a special case, this spec is not totally necessary (sorry) 😊
additional spec move orchestration stack actions update orchestration stacks reference
|
Checked commit jntullo@26ea8ef with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
|
yep, LGTM!! Thanks @jntullo for the enhancement 🍎 |
This adds orchestration stack as a subcollection on services
@miq-bot add_label enhancement, api
@miq-bot assign @abellotti
cc: @AllenBW